refactor!: Rename EditComment to UpdateComment on PullRequestsService, split review comment request bodies, and pass by value - #4493
Open
JamBalaya56562 wants to merge 2 commits into
Conversation
…rvice`, split review comment request bodies, and pass by value CreateComment and EditComment reused the 27-field PullRequestComment response type as their request bodies, and EditComment's doc comment had to warn "A non-nil comment.Body must be provided. Other comment fields should be left nil." The create and update schemas differ — body, commit_id and path are unconditionally required on create while update takes only body — so the body is split into CreatePullRequestCommentRequest (line/start_line/ start_side stay pointers as they are only conditionally required, and the deprecated position parameter is kept with a deprecation note) and UpdatePullRequestCommentRequest. EditComment is renamed to UpdateComment to match the docs operation name. CreateCommentInReplyTo already builds its own minimal body and is unchanged. The PullRequestComment response type stays unchanged, and its entry is removed from the .golangci.yml allowlist. BREAKING CHANGE: PullRequestsService.CreateComment now takes a new CreatePullRequestCommentRequest (with non-pointer Body, CommitID and Path) by value, and PullRequestsService.EditComment is renamed to UpdateComment and takes a new UpdatePullRequestCommentRequest by value, instead of *PullRequestComment.
The pull-request review comment response schema includes body_html, body_text and _links, which were missing from the Go struct. _links is modeled with the new PullRequestCommentLinks type reusing the existing PRLink hypermedia link type.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4493 +/- ##
=======================================
Coverage 98.53% 98.53%
=======================================
Files 195 195
Lines 17691 17691
=======================================
Hits 17431 17431
Misses 260 260 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
gmlewis
added
NeedsReview
gmlewis
approved these changes
Aug 26, 2026
gmlewis
left a comment
gmlewis
left a comment
Collaborator
There was a problem hiding this comment.
Thank you, @JamBalaya56562!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues the request-body-by-value work in #3644, this time for the review comment endpoints on
PullRequestsService— the pulls counterpart of #4444.Commit 1 (
refactor!:) —CreateCommentandEditCommentreused the 27-fieldPullRequestCommentresponse type as their request bodies, andEditComment's doc comment had to warn:Per the docs, the two schemas differ, so the body is split (same approach as #4382/#4401/#4438):
bodycommit_id,pathline,start_line,start_sideside,in_reply_to,subject_typeposition// Deprecated:noteCreatePullRequestCommentRequest—Body/CommitID/Pathare non-pointer since they're unconditionally required; the conditionally-required line fields stay pointers (same treatment astitlein refactor!: RenameNewPullRequesttoCreatePullRequestand pass it by value #4395). Note the request tag isin_reply_to, not the response'sin_reply_to_id.UpdatePullRequestCommentRequest— justBody string.EditCommentis renamed toUpdateCommentto match the docs operation name (same convention as #4444).CreateCommentInReplyToalready builds its own minimal body internally and is unchanged.PullRequestCommentis removed from thebody-allowed-pointer-typesallowlist.Commit 2 (
feat:) — while cross-checking the response schema (prompted by the review feedback on #4444 and #4477, where missing response fields were caught by reviewers), I found three properties missing fromPullRequestCommentand added them up front:BodyHTML,BodyText, andLinks(_links, modeled with a newPullRequestCommentLinkstype reusing the existingPRLink).Verified with
go build ./...,go vet -tags integration ./test/integration/,gofmt, the full./github/test suite (CreateComment/UpdateCommentand the generated accessors at 100%), andcustom-gcl(noparamcheckfindings after removing the allowlist entry).Updates #3644
BREAKING CHANGE: PullRequestsService.CreateComment now takes a new CreatePullRequestCommentRequest (with non-pointer Body, CommitID and Path) by value, and PullRequestsService.EditComment is renamed to UpdateComment and takes a new UpdatePullRequestCommentRequest by value, instead of *PullRequestComment.
cc @jvm986 — flagging for #3644 coordination; this is the
pullscomments type, so no overlap with your recent work.